Skip to content

[VAULT-21456] Add a drop in replacement for the Go regexp package that interns regular expressions#156

Merged
digivava merged 21 commits intomainfrom
sgm/regexp-caching
Feb 24, 2025
Merged

[VAULT-21456] Add a drop in replacement for the Go regexp package that interns regular expressions#156
digivava merged 21 commits intomainfrom
sgm/regexp-caching

Conversation

@kubawi
Copy link
Copy Markdown
Contributor

@kubawi kubawi commented Feb 19, 2025

This PR adds a regexp module, which interns regular expressions to avoid excessive memory use.

The new module uses Go's weak package, so it requires at least Go 1.24 to work.

@kubawi kubawi added the enhancement New feature or request label Feb 19, 2025
@kubawi kubawi self-assigned this Feb 19, 2025
Comment thread regexp/go.mod Outdated
Copy link
Copy Markdown

@bosouza bosouza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, the weak package fits like a glove! I'm so glad we could avoid unsafe/reflection :D

Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp.go
Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp_test.go Outdated
Comment thread regexp/regexp_test.go
Copy link
Copy Markdown

@raskchanky raskchanky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks great! I left a few comments.

As I was reading through this, I thought maybe this was a good use case for https://pkg.go.dev/unique which was introduced in go 1.23. On the surface, it seems like exactly what the unique package was made for. The principal downside to that approach is the pointers aren't weak, so you would end up with a map of regexes that sticks around indefinitely.

Comment thread regexp/regexp.go
Comment thread regexp/regexp.go Outdated
Comment thread regexp/regexp_test.go
@kubawi kubawi marked this pull request as draft February 20, 2025 21:41
… map passed in as an argument and the reverseMap; clean up some comments
…lete entries from either the POSIX or non-POSIX map, rather than both
raskchanky
raskchanky previously approved these changes Feb 21, 2025
Copy link
Copy Markdown

@raskchanky raskchanky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@kubawi kubawi marked this pull request as ready for review February 24, 2025 16:29
Copy link
Copy Markdown
Contributor

@sgmiller sgmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@digivava
Copy link
Copy Markdown
Contributor

Can you add a description to this PR for posterity?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants